Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

api: instance and template details are free text #3240

Merged
merged 12 commits into from
Jun 27, 2019
Merged

api: instance and template details are free text #3240

merged 12 commits into from
Jun 27, 2019

Conversation

rohityadavcloud
Copy link
Member

Problem: Users don't know what keys/values to enter for template and VM details.
Root Cause: The feature does not exist that can list possible details and options.
Solution: Based on the possible VM and template details handled by the
codebase, those details were refactored and a list API is introduced
that can return users those details along with possible values. When
users add details now, they will be presented with a list of key details
and their possible options if any.

Introduces a new API:

listDetailOptions: Lists all possible details and their options for a resource type such as a VM or a template
Required params: resourcetype, 
API Params               Type     Description
==========               ====     ===========
resourceid               string   the UUID of the resource (optional)
resourcetype             string   the resource type such as UserVm, Template etc.

Types of changes

  • Breaking change (fix or feature that would cause existing functionality to change)
  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (improves an existing feature and functionality)
  • Cleanup (Code refactoring and cleanup, that may add test cases)

Screenshots (if appropriate):

Screenshot from 2019-03-27 18-12-18

@rohityadavcloud rohityadavcloud added this to the 4.13.0.0 milestone Mar 28, 2019
@borisstoyanov
Copy link
Contributor

@blueorangutan package

@blueorangutan
Copy link

@borisstoyanov a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

@borisstoyanov
Copy link
Contributor

@rhtyd there's some build failures with this one, can you have a look?

[INFO] 
[INFO] --- maven-clean-plugin:2.5:clean (default-clean) @ cloud-plugin-network-vcs ---
[INFO] Deleting /jenkins/workspace/acs-pr-centos6-pkg-builder/dist/rpmbuild/BUILD/cloudstack-4.13.0.0-SNAPSHOT/plugins/network-elements/brocade-vcs (includes = [target, dist], excludes = [])
[INFO] 
[INFO] --- maven-checkstyle-plugin:3.0.0:check (cloudstack-checkstyle) @ cloud-plugin-network-vcs ---
/opt/rh/maven30/root/usr/bin/mvn: line 9:  2041 Killed                  $M2_HOME/bin/mvn "$@"
error: Bad exit status from /var/tmp/rpm-tmp.ozlF7H (%build)


RPM build errors:
    Bad exit status from /var/tmp/rpm-tmp.ozlF7H (%build)
+ '[' 1 -ne 0 ']'
+ '[' false == true ']'
+ echo 'RPM Build Failed '
RPM Build Failed 
+ exit 3
Build step 'Execute shell' marked build as failure
Stopping all containers
Finished: FAILURE

@blueorangutan
Copy link

Packaging result: ✖centos6 ✔centos7 ✔debian. JID-2653

@rohityadavcloud
Copy link
Member Author

@borisstoyanov looks like an env issue, let's try again
@blueorangutan package

@blueorangutan
Copy link

@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

@blueorangutan
Copy link

Packaging result: ✔centos6 ✔centos7 ✔debian. JID-2657

@borisstoyanov
Copy link
Contributor

@blueorangutan test

@blueorangutan
Copy link

@borisstoyanov a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

borisstoyanov
borisstoyanov requested changes Apr 2, 2019
View reviewed changes
Copy link
Contributor

@borisstoyanov borisstoyanov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rhtyd got some comments for you here, I get this when logged as user:
Screenshot 2019-04-02 at 15 16 16
Perhaps we could think about lining this up with #3213 and to be able properly to filter through the available options. Also user would be able to change/add settings to his VMs so this api should be available.

@blueorangutan
Copy link

Trillian test result (tid-3458)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 37191 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr3240-t3458-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_privategw_acl.py
Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
Smoke tests completed. 69 look OK, 1 have error(s)
Only failed tests results shown below:

Test Result Time (s) Test File
test_04_rvpc_privategw_static_routes Failure 966.29 test_privategw_acl.py

@rohityadavcloud
Copy link
Member Author

Thanks for reporting the issue @borisstoyanov - the issue was missing role permission/authorization for the new API. I've fixed it.
@blueorangutan package

@blueorangutan
Copy link

@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

@blueorangutan
Copy link

Packaging result: ✔centos6 ✔centos7 ✔debian. JID-2676

@rohityadavcloud
Copy link
Member Author

@blueorangutan test

@blueorangutan
Copy link

@rhtyd a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

borisstoyanov
borisstoyanov approved these changes Apr 4, 2019
View reviewed changes
Copy link
Contributor

@borisstoyanov borisstoyanov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM @rhtyd, I wonder how's that going to work with #3244 and #3213, ie is a 'user' going to be able to see all the settings or just the ones he's being eligible to?

@blueorangutan
Copy link

Trillian test result (tid-3469)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 33155 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr3240-t3469-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
Smoke tests completed. 69 look OK, 1 have error(s)
Only failed tests results shown below:

Test Result Time (s) Test File
test_05_rvpc_multi_tiers Failure 413.21 test_vpc_redundant.py
test_05_rvpc_multi_tiers Error 440.47 test_vpc_redundant.py

@rohityadavcloud
Copy link
Member Author

With #3213, users won't be able to see the details admin decides to block. However, it's a presentation limitation only. With this FR, the user is only allowed a set of details that I thought can be allowed in most/all cases. (we can discuss/add/remove the details we want to export or not)

WIth #3244, users will alway see the tab and that some details may always be shown to the user but not allowed to edit. This again is a presentation limitation, a smart use can still use the API/cmk.

@shwstppr shwstppr force-pushed the vm-templates-instance-settings branch from ef16447 to eb5c5b0 Compare May 30, 2019 08:31
@rohityadavcloud
Copy link
Member Author

@blueorangutan package

@blueorangutan
Copy link

@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

@blueorangutan
Copy link

Packaging result: ✔centos6 ✔centos7 ✔debian. JID-2813

@rohityadavcloud
Copy link
Member Author

@blueorangutan test

@blueorangutan
Copy link

@rhtyd a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

@rohityadavcloud
during editing, show all detail options if available
81346bf 
Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
@rohityadavcloud
Copy link
Member Author

Last change is UI only change to show all possible drop down options while editing an existing detail. No smoketests needed.
Screenshot from 2019-06-20 14-24-30

@PaulAngus
Copy link
Member

@blueorangutan package

@blueorangutan
Copy link

@PaulAngus a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

@blueorangutan
Copy link

Packaging result: ✔centos6 ✔centos7 ✔debian. JID-2914

@rohityadavcloud
Copy link
Member Author

@blueorangutan package

@blueorangutan
Copy link

@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

@rohityadavcloud
return rootdisksize detail
2e368c7 
Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
@rohityadavcloud
add osdefault to KVM root disk controller
5d64537 
Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
@rohityadavcloud
remove redundant string
aa5acbc 
Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
@rohityadavcloud
Copy link
Member Author

Fixed few minor issues cc @borisstoyanov @andrijapanic - Kindly kick packaging and build new Trillian env to review. Thanks.

@rohityadavcloud
Copy link
Member Author

@blueorangutan package

@blueorangutan
Copy link

@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

1 similar comment
@blueorangutan
Copy link

@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

@borisstoyanov
Copy link
Contributor

@blueorangutan package

@blueorangutan
Copy link

@borisstoyanov a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

@rohityadavcloud
call the listdetails API before rendering the list view table rows
63c6f31 
Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
@rohityadavcloud
Copy link
Member Author

@blueorangutan package

@blueorangutan
Copy link

@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

@blueorangutan
Copy link

Packaging result: ✔centos6 ✔centos7 ✔debian. JID-14

@rohityadavcloud
Copy link
Member Author

@blueorangutan test

@blueorangutan
Copy link

@rhtyd a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

@rohityadavcloud rohityadavcloud mentioned this pull request Jun 25, 2019
Closed
@blueorangutan
Copy link

Trillian test result (tid-16)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 30315 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr3240-t16-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_privategw_acl.py
Smoke tests completed. 71 look OK, 0 have error(s)
Only failed tests results shown below:

Test Result Time (s) Test File

@rohityadavcloud rohityadavcloud changed the title [WIP DO NOT MERGE] api: instance and template details are free text api: instance and template details are free text Jun 27, 2019
@rohityadavcloud
Copy link
Member Author

Lgtm received from @andrijapanic - can you confirm on the PR. I'll merge this based on testing and review.

@rohityadavcloud rohityadavcloud merged commit 9f4f2c5 into apache:master Jun 27, 2019
@andrijapanicsb
Copy link
Contributor

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DaanHoogland DaanHoogland DaanHoogland left review comments

@nvazquez nvazquez nvazquez left review comments

@borisstoyanov borisstoyanov borisstoyanov approved these changes

@PaulAngus PaulAngus Awaiting requested review from PaulAngus

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
4.13.0.0
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@rohityadavcloud @borisstoyanov @blueorangutan @PaulAngus @andrijapanicsb @DaanHoogland @nvazquez